Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Driver registry #124

Merged
merged 21 commits into from
Mar 24, 2024
Merged

Driver registry #124

merged 21 commits into from
Mar 24, 2024

Conversation

krlmlr
Copy link
Collaborator

@krlmlr krlmlr commented Mar 24, 2024

This PR does many things:

  • Cleanup (before commit "Efficient lookup")
  • Fix how GC is prevented for metadata for registered Arrow tables (was: attributes, now: part of the std::unordered_map<> in C++; before commit "Store config with driver object"
  • Pick up all arguments passed to duckdb() in dbConnect(), and vice versa (before commit "path_normalize")
  • Implement driver registry, closes Reuse database object #123.
  • Tests

With this change, each database file is opened only once, by the call to duckdb() . The database file is closed when the last connection is closed correctly with dbDisconnect(), or when it is garbage-collected (with a warning). The connection still must be closed with dbDisconnect(), but the shutdown argument that was necessary before is now meaningless.

  • Add tests
  • Describe
  • Check consistency of config and readonly objects
  • revdepcheck

@krlmlr krlmlr marked this pull request as draft March 24, 2024 06:47
@krlmlr krlmlr changed the title Store config with driver object Driver registry Mar 24, 2024
@krlmlr krlmlr force-pushed the b-34-56-58-59-60-83-conn-2 branch from 1163852 to ea52aac Compare March 24, 2024 08:20
@codecov-commenter
Copy link

codecov-commenter commented Mar 24, 2024

Codecov Report

Attention: Patch coverage is 94.35484% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 86.28%. Comparing base (210db9b) to head (321cece).

❗ Current head 321cece differs from pull request most recent head 2cab803. Consider uploading reports for the commit 2cab803 to get more accurate results

Files Patch % Lines
src/database.cpp 78.57% 3 Missing ⚠️
R/Driver.R 92.59% 2 Missing ⚠️
src/connection.cpp 80.00% 1 Missing ⚠️
src/include/rapi.hpp 95.23% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #124      +/-   ##
==========================================
+ Coverage   85.87%   86.28%   +0.41%     
==========================================
  Files         107      107              
  Lines        3639     3712      +73     
==========================================
+ Hits         3125     3203      +78     
+ Misses        514      509       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@krlmlr krlmlr force-pushed the b-34-56-58-59-60-83-conn-2 branch 7 times, most recently from 6bcb2e8 to 27c6c4c Compare March 24, 2024 10:09
@krlmlr krlmlr force-pushed the b-34-56-58-59-60-83-conn-2 branch from 27c6c4c to 472f1ef Compare March 24, 2024 10:13
@krlmlr krlmlr force-pushed the b-34-56-58-59-60-83-conn-2 branch 2 times, most recently from a382541 to 7544711 Compare March 24, 2024 14:43
@krlmlr krlmlr force-pushed the b-34-56-58-59-60-83-conn-2 branch from 7544711 to 81bdeb6 Compare March 24, 2024 14:44
@krlmlr
Copy link
Collaborator Author

krlmlr commented Mar 24, 2024

Merging now to get binaries from r-universe. Highly appreciate your review!

@krlmlr krlmlr requested a review from Tmonster March 24, 2024 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reuse database object
3 participants